Skip to content

gh-137574: Add Support For Special Names in help - #137966

Open
adqm wants to merge 35 commits into
python:mainfrom
adqm:pydoc_dunders
Open

adqm wants to merge 35 commits into
python:mainfrom
adqm:pydoc_dunders

Conversation

@adqm

@adqm adqm commented Aug 19, 2025 •

Copy link
Copy Markdown
Contributor

Following on from #137574, this is an attempt to provide better help for special names from the built-in help function.

The original question was about help('__name__') and help('__dict__'), which suggested help for the str and dict built-in types, respectively, instead of reporting an error that no help was available for those topics. I also took this a step further, adding help for the special method names described on the 'Data Model' page of the language reference, as well as '__name__' and '__main__'.

Here's a quick summary of the changes I made:

  • When requesting help on a name '__x__', only report on the result of getattr(builtins, '__x__') if it is a type or a built-in function, preventing the confusion described above.

  • Added help for __name__, __main__, and the special method names as help topics.

  • Added "specialnames" as a new top-level category, and included it as a suggestion if we were looking up a name starting and ending with __.

  • Added a reference to the online Python docs in the message when a help topic isn't found, and slightly changed the wording of that message, hoping not to imply that things that don't have associated help topics aren't documented at all.

If any of these changes is a bridge too far, I can roll them back. Two other concerns, as well:

  • The code style of pydoc.py doesn't match what ruff would output, in a number of places. I opted to mimic the existing code's style rather than using input that ruff would be happy with; I can reformat, though, if that would be better.

  • One of the tests I added (which tests that each dunder method shows up in its help output) is quite slow. I could rework that so that it only tests a few names if that test being slow is a problem.


📚 Documentation preview 📚: https://cpython-previews--137966.org.readthedocs.build/

@AA-Turner

Copy link
Copy Markdown
Member

This PR is way too big. Also, don't regenerate pydoc_data, it's done during the release process.

@ZeroIntensity

Copy link
Copy Markdown
Member

I think the majority of the diff is from pydoc_data.

@AA-Turner

Copy link
Copy Markdown
Member

I'm on mobile so it's hard to tell (the pydoc data diff doesn't even render). I'll look at the PR substantively over the week though.

@adqm

adqm commented Oct 9, 2025

Copy link
Copy Markdown
Contributor Author

@AA-Turner, sorry to pester, and no rush at all, but just wondering if you have a sense of when you might be able to take a look at this PR.

@hugovk
hugovk removed their request for review April 6, 2026 12:20
@github-actions

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label May 15, 2026
@adqm

adqm commented Oct 11, 2026

Copy link
Copy Markdown
Contributor Author

@AA-Turner I know it's been a while, but any chance you could take a look at this PR (and #138095) at some point? I'm happy to make adjustments if necessary.

@ZeroIntensity ZeroIntensity left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think he's been on leave recently, so I can commit to reviewing this instead.

The overall approach here looks good, with just a few small oversights that I noticed. Two side notes:

  1. Let's add an entry to "What's New in Python 3.16".
  2. There are merge conflicts that need to be addressed.

Comment thread Lib/pydoc.py Outdated
Comment thread Lib/pydoc.py
Comment thread Lib/pydoc.py Outdated
Comment thread Misc/NEWS.d/next/Core_and_Builtins/2025-08-19-16-10-26.gh-issue-137966.nEgFAt.rst Outdated
Comment thread Doc/tools/extensions/pydoc_topics.py
Comment thread Lib/pydoc.py
@adqm
adqm requested a review from StanFromIreland as a code owner October 12, 2026 20:10
@bedevere-app bedevere-app Bot added the type-feature A feature request or enhancement label Oct 12, 2026
Comment thread Lib/pydoc_data/topics.py

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please revert this change, it will be done as part of the release.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue is that those changes are necessary for this PR to work, since some of the new topics added to help require topics that aren't currently in there and so would be broken / hard to test. I can revert if the idea is that reviewers will re-generate that file on their own for testing, but just wanted to flag that the PR won't work as expected without it before actually rolling it back.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do have some precedent for regenerating between releases (#153909, #144934, #125074). Is there any harm in doing it here, especially if the feature can't be used without it?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regeneration at release is the standard process (it's automatically done by release-tools), it makes this easier to review IMO.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, but not regenerating makes this very difficult to test locally. How does this make it more difficult to review? Just ignore the regenerated file.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we don't verify it in the CI, it's another file that needs manual verification.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, but manually verifying a generated file is much easier (and less harmful if we mess up) than letting a feature hit main and not work until release. Keep in mind that removing the generated file will also mean we have to remove a bunch of test coverage from this PR. We would, in essence, be trading the guarantee that the generated file is correct for the guarantee that the feature works correctly.

I'm inclined to cite "practicality beats purity" here. Yes, regenerating is a very minor breach of process, but I think reviewing this PR is much more difficult without the regenerated file, not vice versa.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My concern is more around security than correctness here (and not specific to this PR or its author). Since these generated files aren't verified in CI, changes to them in PRs introduce another potential attack surface that reviewers need to manually inspect.

Another downside of regenerating these files outside of releases is the potential for merge conflicts, though I don't think that's much of an issue in the case of pydoc.

That said, I understand the testing concerns, so if you'd like to keep the generated file in and verify the changes, I won't block on it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'm not saying we should make this a common policy; I'm just arguing that we should make an exception for this PR. I'll make sure it's correct before landing this.

Or, we could verify pydoc_data as part of the "Check if generated files are up to date" CI check, assuming the generation is pretty fast.

@read-the-docs-community

read-the-docs-community Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

@adqm

adqm commented Oct 12, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the quick and thorough review, @ZeroIntensity! I added a what's new entry and I believe I fixed the merge conflicts. I responded to some of your specific comments/requests inline above, and of course, happy for more input if you have it.

@adqm

adqm commented Oct 12, 2026

Copy link
Copy Markdown
Contributor Author

Oops, sorry, some of these changes broke some tests. Will ping again when that's resolved. Sorry for the noise.

@adqm

adqm commented Oct 12, 2026

Copy link
Copy Markdown
Contributor Author

The failing test was caused by having two customization labels in the docs (one for basic class customization and the other for run-time config in Doc/using/windows.rst). Having that extra label also broke some internal links in the docs on the tip of main, so for now I just removed that second reference (which didn't seem to be linked from anywhere else), though I suppose we could also consider changing the name of the "Basic Customization" label instead. I'm open to input there.

@ZeroIntensity

Copy link
Copy Markdown
Member

We do our best to avoid removing Sphinx labels when possible (notice the failing CI job).

@adqm

adqm commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor Author

We do our best to avoid removing Sphinx labels when possible (notice the failing CI job).

I understand that, and I agree with the rationale of not breaking links, but I would argue that the addition of that new duplicate reference was a bug, since it is the thing breaking existing links (and it isn't referenced anywhere else in the docs as far as I can see), whereas the old customization label has been there for a while and is linked from elsewhere in the docs.

The new duplicate label looks like it was added in bd31583, per git blame, but that label breaks other links on main and this PR (for example, the link for "Configuration" in the expressions section of the language reference, which was working in the 3.15 docs and which goes back to at least version 3.4).

@ZeroIntensity

Copy link
Copy Markdown
Member

Would you mind opening a separate issue and PR for that? Feel free to tag me for a review.

Comment thread Lib/pydoc.py Outdated
Comment thread Lib/pydoc.py
"""Index of symbols available."""

def bltinlink(name):
return '<a href="topic?key=%s">%s</a>' % (urllib.parse.quote(name), html.escape(name))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

urllib.parse.quote does extra things that seemingly don't work with the .html URL handler (such as converting ! to %21). I think we can switch to html.escape(name) if all we're after is the quote escaping?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was actually originally worried about +, since I know that + is often interpreted as a space in a URL. So I leaned toward urllib.parse.quote just to be safe since I'm not sure what else might be special in the URL bar, and because urllib.parse.quote just felt more right to me in principle, based on the context. I also expect the difference to be largely invisible to the end user, other than slightly-grosser URLs if copy/pasting.

I hadn't noticed any broken pages in my original testing, though I did just now test searching for ! and indeed it shows a bad error message (FWIW, the error is not because of the escaping but because ! doesn't have an entry).

The change I just pushed tries to remedy that by unquoting the given URL on the error page (I was already unquoting when actually doing the lookup, so, e.g., searching for != should have worked before this most recent change).

If you'd prefer, though, I can switch it to use html.escape instead.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would show up if you had a module with some weird Unicode characters in the name. I suppose another fix would be to call urllib.parse.unquote in get_html_page, if you prefer that.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I already have the unquote in the only path in get_html_page where these links could possibly lead (behind the check for if '=' in url, this line in the diff).

I could be missing something, though; is there a case that line doesn't cover that I'm not seeing?

@ZeroIntensity ZeroIntensity removed the stale Stale PR or inactive for long period of time. label Oct 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting review type-feature A feature request or enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants